HIVE-27126: queue level resource stats for YARN RM. - #6501
Conversation
ea3b1b3 to
5f4c748
Compare
9e97b27 to
e2d3173
Compare
e2d3173 to
fa844c0
Compare
fa844c0 to
dac58c2
Compare
dac58c2 to
f4301f1
Compare
f4301f1 to
ada32ce
Compare
8720321 to
827ee45
Compare
827ee45 to
120c749
Compare
120c749 to
34febff
Compare
34febff to
47b3abe
Compare
abstractdog
left a comment
There was a problem hiding this comment.
thanks @architjainjain so far, dropped some comments, but I wasn't able to fully read it through, I'll get back after, in the meantime I can do some testing too hopefully
| * behaviour added as part of HIVE-27126. | ||
| * | ||
| * We capture stdout via a ByteArrayOutputStream and inspect the rendered output. | ||
| */public class TestInPlaceUpdate { |
There was a problem hiding this comment.
line break before public
| for (int i = 0; i < 5; i++) { | ||
| collectors[i] = new YarnQueueMetricsCollector( | ||
| mockYarnClient, "default", refreshIntervalMs, "jitter-test-query-" + i); | ||
| assertNotNull("Collector " + i + " should be created successfully", collectors[i]); | ||
| } | ||
| // If we get here, all 5 collectors were created with their own jittered delays | ||
| // without conflict or exception - thundering herd fix is in place |
There was a problem hiding this comment.
considering mock yarn clients, creating concurrent collectors for jitter testing doesn't seem that valuable to me...I don't think they will ever throw an exception
| mockYarnClient, "default", 30, "circuit-breaker-recovery-query"); | ||
|
|
||
| try { | ||
| // Wait for recovery - snapshot should eventually be populated |
There was a problem hiding this comment.
need extra assertion about the failure first to make 100% sure we actually hit circuit breaker logic
| public void testCircuitBreakerDoesNotAffectSuccessfulCollection() throws Exception { | ||
| // Normal operation - no failures, circuit breaker should never activate |
There was a problem hiding this comment.
this is just a simple happy testing, I guess it's covered by testSuccessfulMetricsCollection
There was a problem hiding this comment.
@abstractdog Thanks for catching this! I've added JavaDoc to clarify that this test is part of the circuit breaker test suite. While it looks similar to testSuccessfulMetricsCollection(), it specifically validates that the circuit breaker doesn't interfere with normal operations. The circuit breaker suite needs three scenarios: failure, recovery, and happy path. This test covers the happy path behavior for circuit breaker specifically, not general metrics collection. I've added cross-references to the related tests for clarity.
| when(mockQueueStats.getAllocatedMemoryMB()).thenReturn(1024L); | ||
| when(mockQueueStats.getAvailableMemoryMB()).thenReturn(1024L); | ||
| when(mockQueueStats.getAllocatedVCores()).thenReturn(4L); | ||
| when(mockQueueStats.getAvailableVCores()).thenReturn(4L); | ||
| when(mockQueueStats.getNumAppsRunning()).thenReturn(1L); | ||
| when(mockQueueStats.getPendingContainers()).thenReturn(0L); | ||
| when(mockQueueInfo.getQueueStatistics()).thenReturn(mockQueueStats); | ||
| when(mockQueueInfo.getCapacity()).thenReturn(0.5f); | ||
| when(mockYarnClient.getQueueInfo("default")).thenReturn(mockQueueInfo); |
There was a problem hiding this comment.
I feel like defining all these variables adds a lot of boilerplate to the unit tests, whereas in this case we're only interested in collection timestamp, could a separate method be created just for adding happy value, and it can be overridden per test, like:
when(mockQueueStats.getAllocatedMemoryMB()).thenReturn(1024L);
when(mockQueueStats.getAvailableMemoryMB()).thenReturn(1024L);
when(mockQueueStats.getAllocatedVCores()).thenReturn(4L);
when(mockQueueStats.getAvailableVCores()).thenReturn(4L);
when(mockQueueStats.getNumAppsRunning()).thenReturn(1L);
when(mockQueueStats.getPendingContainers()).thenReturn(0L);
when(mockQueueInfo.getQueueStatistics()).thenReturn(mockQueueStats);
when(mockQueueInfo.getCapacity()).thenReturn(0.5f);
when(mockYarnClient.getQueueInfo("default")).thenReturn(mockQueueInfo);
method javadoc can also tell that these are only default happy values for the unit tests
| // HIVE-27126: Thrift regeneration omitted setStartTimeIsSet(true) from the constructor. | ||
| // Explicitly call setStartTime() to set the isset flag required for Thrift validation. | ||
| tProgressUpdateResp.setStartTime(progressUpdate.startTimeMillis); | ||
| if (progressUpdate.queueMetrics() != null && !progressUpdate.queueMetrics().isEmpty()) { | ||
| tProgressUpdateResp.setQueueMetrics(progressUpdate.queueMetrics()); | ||
| } | ||
| resp.setProgressUpdateResponse(tProgressUpdateResp); |
There was a problem hiding this comment.
why is this change needed?
There was a problem hiding this comment.
This works around a Thrift code generation bug. When we regenerated Thrift code after adding the queueMetrics field, the generated constructor for TProgressUpdateResp accepts startTimeMillis but fails to set the isset flag. Without this flag, Thrift serialization treats the field as unset, causing clients to receive incomplete progress updates. The explicit setStartTime() call ensures both the value AND the isset flag are properly set, maintaining backward compatibility with Thrift clients.
without this test the generated file is not having the starttime isset flag updated.
| this.status = status; | ||
| this.footerSummary = footerSummary; | ||
| this.startTime = startTime; | ||
| setStartTimeIsSet(true); |
There was a problem hiding this comment.
@abstractdog this is getting removed. so we added manually after the constructor call.
47b3abe to
e7a0726
Compare
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds optional, real-time YARN queue resource metrics to the Tez progress display during Hive query execution, propagating the data through HS2/Thrift and Beeline in-place updates.
Changes:
- Introduces YARN queue metrics collection (collector + per-queue state + shared cache + scheduled refresh pool with jitter/circuit breaker).
- Integrates queue metrics into Tez progress rendering (in-place + log-to-file) and publishes it over Thrift (
TProgressUpdateResp). - Adds unit tests covering cache/state/pool behavior and Tez monitor formatting/initialization.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| service/src/java/org/apache/hive/service/server/HiveServer2.java | Initializes the shared refresh pool during HS2 Tez session pool startup. |
| service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java | Populates new Thrift queueMetrics field and applies Thrift “isset” workaround for startTime. |
| service/src/java/org/apache/hive/service/cli/JobProgressUpdate.java | Adds queue metrics string to progress update model. |
| service-rpc/if/TCLIService.thrift | Adds optional queueMetrics to TProgressUpdateResp. |
| ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java | Implements queueMetrics() default for the progress monitor facade. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/QueueMetricsCollector.java | New interface for queue metrics collectors. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/NoOpQueueMetricsCollector.java | Null-object collector when feature is disabled. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/QueueMetricsSnapshot.java | Immutable snapshot of queue resource stats fetched from RM. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/QueueMetricsState.java | Per-queue state: interval registration, scheduling, refresh lock, circuit breaker. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/QueueMetricsCache.java | JVM-wide Guava cache mapping queue name → state with expiry. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/QueueMetricsRefreshPool.java | JVM-wide scheduled executor singleton to run refresh tasks (with jitter). |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/YarnQueueMetricsCollector.java | Active collector that coordinates with shared cache/state and refresh pool. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/TezProgressMonitor.java | Formats and returns multi-line queue metrics for progress rendering. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/TezJobMonitor.java | Creates/shuts down the metrics collector based on config and wires it into progress monitor. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/RenderStrategy.java | Appends queue metrics into log-to-file progress report (single-line rendering). |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java | Adds per-session YarnClient lifecycle to support collector RM queries. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java | Exposes YarnClient via pooled session wrapper. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSession.java | Extends TezSession interface with getYarnClient(). |
| common/src/java/org/apache/hadoop/hive/conf/HiveConf.java | Adds new configuration keys for refresh interval and refresh thread pool size. |
| common/src/java/org/apache/hadoop/hive/common/log/ProgressMonitor.java | Adds queueMetrics() to the ProgressMonitor contract. |
| common/src/java/org/apache/hadoop/hive/common/log/InPlaceUpdate.java | Renders queue metrics block (multi-line) under the standard progress output. |
| beeline/src/java/org/apache/hive/beeline/logs/BeelineInPlaceUpdateStream.java | Reads queueMetrics from Thrift progress updates for in-place rendering. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/monitoring/yarnqueue/TestQueueMetricsState.java | Unit tests for per-queue state logic (intervals, refresh lock, circuit breaker). |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/monitoring/TestTezProgressMonitorQueueMetrics.java | Unit tests for queue metrics formatting and edge cases in TezProgressMonitor. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/monitoring/TestTezJobMonitorQueueMetrics.java | Unit tests for TezJobMonitor collector initialization decisions. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestYarnQueueMetricsCollector.java | Unit tests for collector/cache/pool integration behavior and circuit breaker. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestQueueMetricsRefreshPool.java | Unit tests for refresh pool singleton, scheduling, and jitter determinism/range. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestQueueMetricsCache.java | Unit tests for cache placeholder/put semantics and concurrency behavior. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestNoOpQueueMetricsCollector.java | Unit tests for NoOp collector behavior and singleton semantics. |
Files not reviewed (1)
- service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TProgressUpdateResp.java: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (session != null && yarnClient == null) { | ||
| try { | ||
| yarnClient = YarnClient.createYarnClient(); | ||
| yarnClient.init(conf); | ||
| yarnClient.start(); | ||
| LOG.info("YarnClient initialized for session: {}", sessionId); | ||
| } catch (Exception e) { | ||
| LOG.warn("Failed to initialize YarnClient for metrics collection", e); | ||
| yarnClient = null; | ||
| } | ||
| } |
| public static long calculateJitter(String queueName, long intervalMs) { | ||
| long jitterWindow = intervalMs * JITTER_PERCENT / 100; | ||
| return Math.abs(queueName.hashCode()) % jitterWindow; | ||
| } |
| public ScheduledFuture<Void> scheduleRefreshTask(Runnable task, long intervalMs) { | ||
| return (ScheduledFuture<Void>) refreshPool.scheduleWithFixedDelay(task, 0, intervalMs, TimeUnit.MILLISECONDS); | ||
| } |
| /** | ||
| * Initializes the queue metrics refresh pool and HTTP exporter. | ||
| * Failures are non-fatal — logged as warnings so the server can start without queue metrics. | ||
| */ | ||
| private void initializeQueueMetricsPool(HiveConf hiveConf) { |
| } catch (Exception e) { | ||
| LOG.warn("Failed to initialize queue metrics refresh pool: {}", e.getMessage()); | ||
| } |
| private static final Logger LOG = LoggerFactory.getLogger(QueueMetricsRefreshPool.class); | ||
|
|
||
| private static final int DEFAULT_THREAD_COUNT = 4; | ||
| public static final int JITTER_PERCENT = 10; |
| HIVE_TEZ_QUEUE_METRICS_REFRESH_INTERVAL("hive.tez.queue.metrics.refresh.interval", "0s", | ||
| new TimeValidator(TimeUnit.SECONDS), | ||
| "Interval for refreshing YARN queue resource metrics during Tez query execution. " + | ||
| "When set to a positive value (e.g. 10s), displays real-time memory, vCore, capacity " + | ||
| "and application metrics for the YARN queue being used. " + | ||
| "Set to 0 or negative to disable. Minimum effective value is 1 second."), |
| long startTime = System.currentTimeMillis(); | ||
| QueueMetricsSnapshot snapshot; | ||
| while ((snapshot = collector.getLatestSnapshot()) == null) { | ||
| if (System.currentTimeMillis() - startTime > timeoutMs) { | ||
| fail("Snapshot not available after " + timeoutMs + "ms"); | ||
| } | ||
| } | ||
| return snapshot; |
| for (Object[] row : cases) { | ||
| String label = (String) row[0]; | ||
| YarnClient yarnClient = (YarnClient) row[1]; | ||
| String queueName = (String) row[2]; | ||
|
|
||
| // Re-initialise mocks so state from the previous row does not leak. | ||
| MockitoAnnotations.openMocks(this); | ||
| when(mockDag.getName()).thenReturn("test-dag-1"); |



What changes were proposed in this pull request?
This PR adds real-time YARN queue resource metrics display alongside Tez job progress during query execution. Users can now see memory, vCores, capacity utilization, running/pending applications, and allocated/pending containers for the queue being used by their queries.
Key Components:
What is NOT Changed:
Technical Highlights:
Configuration:
hive.tez.queue.metrics.refresh.interval: Per-session refresh interval (default: 10s, set to 0s to disable)hive.server2.tez.queue.metrics.refresh.threads: Pool size for metric collection (default: 4)Important Note: An HTTP exporter and Prometheus/Grafana integration were implemented as a local testing setup to validate the feature during development. These components are NOT part of this PR, NOT included in production code, and were only used in a local Docker environment for validation purposes.
Why are the changes needed?
Currently, when a Hive query is slow or stalled, users cannot determine if the issue is due to insufficient queue resources or other factors. They see Tez task progress (e.g., "Map 1: 3/10") but have no visibility into whether the queue has available memory/vCores to execute tasks in parallel.
This enhancement provides real-time queue resource information, enabling users to:
Backward Compatibility:
Performance Impact:
Does this PR introduce any user-facing change?
Yes. When
hive.tez.queue.metrics.refresh.intervalis set to a positive value (default: 10s), users will see queue-level metrics displayed with Tez job progress:In-place mode (hive.server2.in.place.progress=true):
Log file mode (hive.server2.in.place.progress=false):
When disabled (set hive.tez.queue.metrics.refresh.interval=0s):
No queue metrics are displayed, behavior remains identical to previous versions.
How was this patch tested?
The patch was tested in a multi-node YARN cluster environment with concurrent queries running on different queues.
Testing included:
Test Environment:
Validation artifacts (local testing setup only - NOT in production):
Screenshots:
1. Terminal output with queue metrics enabled (hive.tez.queue.metrics.refresh.interval=10s):
2. Terminal output with queue metrics disabled (hive.tez.queue.metrics.refresh.interval=0s):
3. Grafana dashboard showing backend metrics validation (local testing setup only):
Note: The Grafana and Prometheus infrastructure shown above was created exclusively for local testing and validation purposes. This monitoring stack is NOT part of the production code, NOT included in the deployment, and was only used in a local Docker environment to validate the feature's correctness.
Configuration tested: